utils.network.hosts: Make subclass of Host instantiable #5714
Conversation
|
The changes look good to me, however, I'd prefer a clearer description in commit summary and message. |
avocado/utils/network/hosts.py
Outdated
| @@ -48,7 +48,7 @@ class Host: | |||
| """ | |||
|
|
|||
| def __init__(self, host): | |||
| if isinstance(self, Host): | |||
| if type(self) == Host: | |||
There was a problem hiding this comment.
Btw, there is another approach to avoid performing this check by invoking type directly, that is we could make the Host be an abstract class with its __init__ method being an abstract method.
$ cat /tmp/t.py
from abc import ABCMeta, abstractmethod
class Foo(metaclass=ABCMeta):
@abstractmethod
def __init__(self, host):
self.host = host
class Bar(Foo):
def __init__(self, host='localhost'):
super().__init__(host)
b = Bar()
print(b.host)
f = Foo('localhost')
$ python3 /tmp/t.py
localhost
Traceback (most recent call last):
File "/tmp/t.py", line 19, in <module>
f = Foo('localhost')
^^^^^^^^^^^^^^^^
TypeError: Can't instantiate abstract class Foo with abstract method __init__
yanglei-rh
changed the title
|
Hello @clebergnu Could you please help me review this patch? Thanks in advance. |
yanglei-rh
changed the title
|
Test Pass: |
There was a problem hiding this comment.
Hello @clebergnu Could you please help me review this patch? Thanks in advance.
@yanglei-rh the change itself looks good. I apologize introducing the breakage.
In order to silence pylint, and make CI checks pass, please add:
diff --git a/avocado/utils/network/hosts.py b/avocado/utils/network/hosts.py
index 0a1c2add3..7d61ad640 100644
--- a/avocado/utils/network/hosts.py
+++ b/avocado/utils/network/hosts.py
@@ -48,7 +48,7 @@ class Host:
"""
def __init__(self, host):
- if type(self) == Host:
+ if type(self) == Host: # pylint: disable=C0123
raise TypeError("Host class should not be instantiated")
self.host = host
Thanks for your help review. After silence pylint, CI checks pass. |
|
Hi @luckyh Could you please help me review it again and merge this MR? Thanks a lot. |
@yanglei-rh can you please address CI issue , as have space around #pylint comment |
A regression bug was introduced by 94fb7a8,which results Host's subclass LocalHost can not be instantiated. Existing instantiation methods the same as "localhost = LocalHost()" will no longer fail after changed. Signed-off-by: Lei Yang <[email protected]>
Hi @PraveenPenguin Thanks for your reminder, this problem has been fixed, please help review it again, thanks a lot. |
A regression bug was introduced by 94fb7a8,which results Host's subclass LocalHost can not be instantiated. Existing instantiation methods the same as "localhost = LocalHost()" will no longer fail after changed.
ID:2219366
Signed-off-by: Lei Yang [email protected]